-
Notifications
You must be signed in to change notification settings - Fork 115
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
update scope of fixtures from hosts-content longrun test_inc_updates #14842
update scope of fixtures from hosts-content longrun test_inc_updates #14842
Conversation
trigger: test-robottelo |
PRT Result
|
c526c4e
to
dfc3e15
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code looks good to me. The test failure is kind of weird, looks like a redirect loop for redhat.com? I can't reproduce this locally. I'll try rerunning the 3.12 code quality check.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice fix! I could not reproduce either
@@ -231,7 +231,7 @@ def test_positive_noapply_api( | |||
|
|||
|
|||
@pytest.mark.tier3 | |||
def test_positive_incremental_update_time(module_target_sat, module_sca_manifest_org): | |||
def test_positive_incremental_update_time(target_sat, function_sca_manifest_org): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are you using sca enabled manifest?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- During 6.14.z TFA, I find this test was failing due to reimporting of manifest in same orgnaization (created using module scope fixture)
- I just want to keep
*_sca_manifest_org
as it is, so just changed scope of fixture.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@vijaysawant I had the same question while reading this, but this issue should be mentioned in the PR description in problem/solution.
And could you be little more descriptive in PR title and commit messge, which could help you in future to identity the cause of this failures?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure @Gauravtalreja1 , this PR initially open for testing purpose and wanted to see behaviour of when test running in Jenkins (as same changes/ same tests in 6.14
& 6.15
behave differently if I talk about test results from latest build, we @ColeHiggins2 and @vsedmik monitor robottelo logs closely and took decision to test changes remotely)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My point was if we use non-sca function scoped fixture then also it would have worked.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's fine by me
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jameerpathan111 @shweta83, from what I understood from the subscription team, we should move away from the entitlement manifests/orgs rather sooner than later. So, if there is no specific reason to use the entitlement manifest (e.g. testing subscription attach etc.), then we should rather go SCA.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change was going on 6.14.z branch so to keep the things consistent, I suggested the above change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@vsedmik I understand but my main point was using a module-level fixture in both the tests, whether it's module_sca_manifest_org
or module_entitlement_manifest_org
. Anyway, it was more of a suggestion and I'm fine with the current approach too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jameerpathan111 due to module scope fixture test setup fails- because it creates organization and reuse in another test, the second test case uploads newly created manifest and due to which it end up with en Error
['Owner has already imported from another subscription management application. The following conflicts were found: [ DISTRIBUTOR_CONFLICT ]']
May be I unable to explain it in correct way, @synkd @vsedmik @ColeHiggins2 were discussing this issue last week, so we took dicision to change scope of fixture (module to function) rather change functionality (sca to non-sca)
dfc3e15
to
c4554ef
Compare
trigger: test-robottelo |
PRT Result
|
Problem Statement
longrun/test_inc_updates.py
has 2 tests, module scope fixture creates one organization and same organization has been used in two test cases, after uploading manifest second time test was failing due to below error messagetest name: test_positive_incremental_update_time
Error:
['Owner has already imported from another subscription management application. The following conflicts were found: [ DISTRIBUTOR_CONFLICT ]']
Solution
changed scope of fixture of one test case so it will not use previously created organization from same test session.
Related Issues
N/A
PRT test Cases example
trigger: test-robottelo
pytest: tests/foreman/longrun/test_inc_updates.py